-
Couldn't load subscription status.
- Fork 1.9k
fix(schema-compiler): add support for time filters and rolling windows and fix subquery aliasing in Oracle dialect #10066
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(schema-compiler): add support for time filters and rolling windows and fix subquery aliasing in Oracle dialect #10066
Conversation
… for Oracle compatibility Oracle database does not support the AS keyword for table/subquery aliasing, while other databases like PostgreSQL and MySQL do. The existing BaseQuery implementation hardcoded 'as' in subquery alias generation, causing Oracle queries to fail. This change: - Replaces hardcoded 'as' with asSyntaxJoin property in BaseQuery - Oracle returns empty string for asSyntaxJoin (no AS keyword) - PostgreSQL/MySQL return 'AS' (maintains existing behavior) - Adds comprehensive Oracle query test suite validating AS syntax removal - Adds PostgreSQL regression test ensuring AS keyword is still present This fixes queries with rolling windows and multiple subqueries on Oracle, which previously generated invalid SQL like: SELECT ... FROM (...) as q_0 INNER JOIN (...) as q_1 ON ... Now Oracle correctly generates: SELECT ... FROM (...) q_0 INNER JOIN (...) q_1 ON ...
- Fix AS keyword in subquery aliases (Oracle doesn't support it) - Handle time dimensions without granularity to prevent TypeError - Implement Oracle-specific interval arithmetic using ADD_MONTHS and NUMTODSINTERVAL - Add comprehensive test suite for Oracle query generation These changes enable Oracle users to execute queries with: - Time dimension filters without granularity specification - Rolling windows and time-based calculations - Multiple subqueries with proper aliasing All changes maintain backward compatibility with other database adapters.
|
@Tankilevitch thank you for the fix! I'll review it a bit later. |
packages/cubejs-schema-compiler/test/unit/postgres-query.test.ts
Outdated
Show resolved
Hide resolved
…cle-subquery-as-syntax
Oracle's custom groupByClause() was incorrectly including time dimensions without granularity in the GROUP BY clause, causing SQL errors when time dimensions were used solely for filtering (no grouping). Changes: - Update OracleQuery.groupByClause() to check selectColumns() before including dimensions in GROUP BY - Time dimensions without granularity return null from selectColumns(), so they are now properly excluded - Add comprehensive tests for both PostgreSQL and Oracle covering: * Time dimensions without granularity (filtering only) * Time dimensions with granularity (grouping and filtering) - Remove unnecessary guard in BaseQuery.dimensionTimeGroupedColumn() as it's no longer needed with the Oracle-specific fix The fix aligns Oracle's behavior with PostgreSQL, which already handles this correctly through its base implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻 LGTM!
Fix Oracle Query Generation Issues
Summary
This PR fixes three critical Oracle database compatibility issues that prevented queries with rolling windows, time dimension filters, and subqueries from executing. All fixes maintain backward compatibility with other database adapters.
Issue #1: Invalid
ASKeyword in Subquery AliasesProblem
Oracle does not support the
ASkeyword before table/subquery aliases. Queries requiring subqueries would fail with:Why It Happened
The query builder was hardcoding the
askeyword when generating subquery aliases, which works for PostgreSQL/MySQL but is invalid in Oracle SQL syntax.What We Did
Modified the query builder to use a database-specific property (
asSyntaxJoin) that each adapter can override. Oracle sets this to an empty string, while other databases use"AS".Example
PostgreSQL (unchanged):
Oracle (fixed):
Issue #2: TypeError on Time Dimensions Without Granularity
Problem
When using time dimensions for filtering only (without specifying granularity), queries would crash with:
Example Query
{ "measures": ["visitors.count"], "timeDimensions": [{ "dimension": "visitors.createdAt", "dateRange": ["2020-01-01", "2020-12-31"] // No granularity - just filtering }] }Why It Happened
The code assumed granularity was always present and tried to access properties on an undefined object. Time dimensions used only for filtering don't require granularity.
What We Did
Added a null check before accessing granularity properties. When granularity is not specified, the dimension is used as-is for filtering without grouping.
Result
Issue #3: Invalid Interval Syntax for Oracle
Problem
Queries with rolling windows would fail with:
Example Query
{ "measures": ["visitors.unboundedCount"], "timeDimensions": [{ "dimension": "visitors.createdAt", "granularity": "year", "dateRange": ["2020-01-01", "2022-12-31"] }] }(where
unboundedCounthas arollingWindowconfiguration)Why It Happened
The default date arithmetic used PostgreSQL-style interval syntax:
Oracle requires specific functions for date arithmetic instead of the
INTERVALkeyword.What We Did
Implemented Oracle-specific
addIntervalandsubtractIntervalmethods that use:ADD_MONTHS(date, n)for year/month/quarter intervalsNUMTODSINTERVAL(n, unit)for day/hour/minute/second intervalsTransformation Examples
Before (PostgreSQL syntax):
After (Oracle syntax):
Interval Conversion
ADD_MONTHS(date, ±12)ADD_MONTHS(date, ±3)ADD_MONTHS(date, ±1)date ± NUMTODSINTERVAL(n, 'DAY')date ± NUMTODSINTERVAL(n, 'HOUR')Testing
New Test Coverage
Created comprehensive test suite in
oracle-query.test.tswith 10 tests covering:ASkeyword (multiple scenarios)FETCH NEXTsyntax instead ofLIMITRegression Protection
Added test in
postgres-query.test.tsto ensure PostgreSQL continues usingASkeyword correctly.Backward Compatibility
✅ All changes are backward compatible:
ASkeywordImpact
These fixes enable Oracle users to: